Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Use enum class for MaterialUpdateStage #1205

Merged
merged 15 commits into from
Mar 30, 2022
Merged

refactor: Use enum class for MaterialUpdateStage #1205

merged 15 commits into from
Mar 30, 2022

Conversation

paulgessinger
Copy link
Member

@paulgessinger paulgessinger commented Mar 24, 2022 •
edited
Loading

This change is motivated by arithmetic between non-class enums and other types that are not the same enum being deprecated in C++20. This mainly affects NavigationDirection, but I took the chance to update the MaterialUpdateStage enum to make it an enum class.

@paulgessinger paulgessinger added this to the next milestone Mar 24, 2022
@codecov
Copy link

codecov bot commented Mar 25, 2022 •
edited
Loading

Codecov Report

Merging #1205 (981ec79) into main (f7c32a8) will increase coverage by 0.00%.
The diff coverage is 44.00%.

@@           Coverage Diff           @@
##             main    #1205   +/-   ##
=======================================
  Coverage   47.85%   47.86%           
=======================================
  Files         371      372    +1     
  Lines       19413    19443   +30     
  Branches     9148     9151    +3     
=======================================
+ Hits         9291     9306   +15     
- Misses       3791     3806   +15     
  Partials     6331     6331
Impacted Files Coverage Δ
Core/src/Definitions/Common.cpp 0.00% <0.00%> (ø)
...de/Acts/TrackFinding/CombinatorialKalmanFilter.hpp 33.24% <37.50%> (+0.26%) ⬆️
Core/include/Acts/TrackFitting/KalmanFitter.hpp 44.48% <50.00%> (+0.91%) ⬆️
Core/include/Acts/TrackFitting/detail/GsfActor.hpp 42.02% <57.14%> (+0.68%) ⬆️
Core/include/Acts/Material/ISurfaceMaterial.hpp 70.83% <100.00%> (+7.67%) ⬆️
...Propagator/detail/PointwiseMaterialInteraction.hpp 96.87% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

stephenswat
stephenswat approved these changes Mar 25, 2022
View reviewed changes
Copy link
Member

@stephenswat stephenswat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one looks good to go as it - there is one thing I would change, but it's more of an opinion matter, so I'll stamp it and then you can decide where to go with it. 👍

Core/src/Definitions/Common.cpp Show resolved Hide resolved
Core/include/Acts/Material/ISurfaceMaterial.hpp Outdated Show resolved Hide resolved
@stephenswat stephenswat added Component - Core Affects the Core module Improvement Changes to an existing feature Impact - Minor Nuissance bug and/or affects only a single module labels Mar 25, 2022
stephenswat
stephenswat approved these changes Mar 28, 2022
View reviewed changes
Copy link
Member

@stephenswat stephenswat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks!

@paulgessinger
Copy link
Member Author

Updated after GSF went in, @stephenswat can you reapprove?

@paulgessinger
Copy link
Member Author

Actually, I now also removed the arithmetic operator overload, since that's not needed anymore after the change requested by @stephenswat.

stephenswat
stephenswat approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@stephenswat stephenswat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kodiakhq kodiakhq bot merged commit 8016339 into acts-project:main Mar 30, 2022
@paulgessinger paulgessinger removed this from the next milestone Apr 4, 2022
@paulgessinger paulgessinger added this to the v18.0.0 milestone Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephenswat stephenswat stephenswat approved these changes

Assignees
No one assigned
Labels
automerge Component - Core Affects the Core module Impact - Minor Nuissance bug and/or affects only a single module Improvement Changes to an existing feature
Projects
None yet
Milestone
v18.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@paulgessinger @stephenswat